fix(nodejs): add CJS compatibility for VS Code extensions by darthmolen · Pull Request #546 · github/copilot-sdk

darthmolen · 2026-02-23T18:45:52Z

Summary

Fixes #528 — getBundledCliPath() uses import.meta.resolve("@github/copilot/sdk") which throws in CJS contexts (VS Code extensions bundled with esbuild format:"cjs").

Changes:

Replace import.meta.resolve with createRequire + module resolution path walking (works in both ESM and CJS)
Add bundled CJS output (dist/cjs/index.cjs) via esbuild
Add conditional exports in package.json (import + require)
Add CJS compatibility test suite (4 tests)

Problem

The @github/copilot package only exposes an ESM "import" condition for its ./sdk subpath export — no "require" condition. This means:

import.meta.resolve("@github/copilot/sdk") — works in ESM only
require.resolve("@github/copilot/sdk") — fails (Package subpath './sdk' is not defined by "exports")
require.resolve("@github/copilot/package.json") — also fails (not exported)

When a consumer bundles the SDK with esbuild format:"cjs", import.meta becomes an empty object, and import.meta.resolve is undefined.

Solution

Instead of resolving through the @github/copilot package's strict exports, we:

Use createRequire(import.meta.url ?? pathToFileURL(__filename).href) — import.meta.url in ESM, __filename fallback in CJS
Walk require.resolve.paths("@github/copilot") to get the Node module search directories
Check each path for @github/copilot/index.js using existsSync

This avoids the ESM-only exports entirely and works in both module systems.

Test plan

Existing unit tests pass (19/19 passing, 3 pre-existing CLI spawn failures unchanged)
New CJS tests pass (4/4):
- CJS dist file exists
- ESM dist file exists
- CJS build is requireable and exports CopilotClient
- CopilotClient constructor resolves bundled CLI path in CJS context
Manual: bundle SDK in a VS Code extension with esbuild format:"cjs", verify getBundledCliPath() works

Copilot

Pull request overview

This PR fixes a critical CJS compatibility issue (#528) where getBundledCliPath() failed in VS Code extensions bundled with esbuild format:"cjs". The root cause was using import.meta.resolve(), an ESM-only API that becomes undefined in CJS contexts.

Changes:

Replaced import.meta.resolve() with a createRequire() + path walking approach that works in both ESM and CJS environments
Added CJS build output (dist/cjs/index.cjs) via esbuild bundling
Updated package.json with conditional exports supporting both import and require conditions
Added 4 new CJS compatibility tests to verify the fix

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File	Description
nodejs/src/client.ts	Refactored `getBundledCliPath()` to use `createRequire()` with `__filename` fallback instead of `import.meta.resolve()`, making it work in CJS bundles
nodejs/package.json	Added conditional exports with separate `import` (ESM) and `require` (CJS) entry points, updated `main` field to point to ESM build
nodejs/esbuild-copilotsdk-nodejs.ts	Added CJS build configuration that bundles to `dist/cjs/index.cjs` with external dependencies marked
nodejs/test/cjs-compat.test.ts	New test suite with 4 tests validating CJS build exists, is requireable, exports CopilotClient, and resolves CLI path correctly

Copilot · 2026-02-23T18:50:40Z

nodejs/test/cjs-compat.test.ts

+        // Verify that new CopilotClient({ cliUrl: "8080" }) doesn't throw
+        // (constructor calls getBundledCliPath() for the default cliPath)
+        const script = `
+            const sdk = require(${JSON.stringify(cjsEntryPoint)});
+            // Use cliUrl to avoid actually spawning the CLI,
+            // but the constructor still evaluates getBundledCliPath() for cliPath default


The test comment states that using cliUrl: "8080" avoids spawning the CLI but "the constructor still evaluates getBundledCliPath() for cliPath default". However, looking at the constructor implementation, when cliUrl is provided, the cliPath is still resolved via getBundledCliPath() even though it won't be used (since isExternalServer will be true).

This test correctly validates that getBundledCliPath() works in a CJS context, but it may not catch scenarios where the @github/copilot package is not installed or available in node_modules, which is common in VS Code extension bundles that don't include node_modules. Consider adding a test case that simulates this scenario or clarifying in the comment that the test assumes @github/copilot is installed in node_modules.

Suggested change

// Verify that new CopilotClient({ cliUrl: "8080" }) doesn't throw

// (constructor calls getBundledCliPath() for the default cliPath)

const script = `

const sdk = require(${JSON.stringify(cjsEntryPoint)});

// Use cliUrl to avoid actually spawning the CLI,

// but the constructor still evaluates getBundledCliPath() for cliPath default

// Verify that new CopilotClient({ cliUrl: "8080" }) doesn't throw.

// In the current implementation, the constructor still resolves a default

// cliPath via getBundledCliPath(), even when cliUrl is provided, so this

// test exercises getBundledCliPath() in a pure CJS context.

//

// NOTE: This test assumes that the bundled @github/copilot CLI package is

// present in node_modules. It does NOT simulate environments (e.g. some

// VS Code extension bundles) where @github/copilot is absent.

const script = `

const sdk = require(${JSON.stringify(cjsEntryPoint)});

// Use cliUrl to avoid actually spawning the CLI process; we only care

// that constructing the client (and resolving the bundled CLI path)

// does not throw in a CJS-only environment.

Copilot · 2026-02-23T18:50:40Z

nodejs/src/client.ts

+    const searchPaths = require.resolve.paths("@github/copilot") ?? [];
+    for (const base of searchPaths) {
+        const candidate = join(base, "@github", "copilot", "index.js");
+        if (existsSync(candidate)) {
+            return candidate;
+        }
+    }
+    throw new Error(
+        "Could not find @github/copilot package. Ensure it is installed."
+    );
 }


The implementation assumes that require.resolve.paths("@github/copilot") will return a non-empty array. However, in some bundled scenarios or when using alternative package managers, this method might return null or an empty array, which would cause the function to throw immediately without providing helpful debugging information.

Consider adding logging or a more descriptive error message that explains what paths were searched. For example:

const searchPaths = require.resolve.paths("@github/copilot") ?? []; if (searchPaths.length === 0) { throw new Error( "Could not determine module resolution paths for @github/copilot. " + "Ensure the package is installed or provide an explicit cliPath option." ); } // ... existing loop ...

This would help developers diagnose issues more quickly.

Copilot · 2026-02-23T18:50:40Z

nodejs/esbuild-copilotsdk-nodejs.ts

+    // Mark dependencies as external — they're resolved at runtime by the consumer
+    external: [
+        "vscode-jsonrpc",
+        "vscode-jsonrpc/*",
+        "zod",
+        "@github/copilot",
+        "@github/copilot/*",
+    ],


The CJS build configuration marks @github/copilot and @github/copilot/* as external dependencies, which means they won't be bundled. This is correct since getBundledCliPath() needs to resolve the actual installed package in node_modules at runtime.

However, this creates a potential issue: if a consumer (like a VS Code extension) bundles the SDK and doesn't include @github/copilot in their node_modules at runtime, getBundledCliPath() will fail. The PR description mentions this scenario but doesn't provide a clear solution beyond passing an explicit cliPath.

Consider documenting this requirement prominently in the README or adding a fallback behavior when the package can't be found.

SteveSandersonMS · 2026-02-23T18:53:38Z

Thanks @darthmolen. I was expecting we could produce just a single build output. Supporting both cjs and esm will be an ongoing cost as people would encounter different issues across the two. Do you have a reason to think we need to support both and not just CJS?

SteveSandersonMS · 2026-02-23T18:55:18Z

Perhaps the best outcome altogether would be having only a single ESM build, but putting in place whatever fallback logic is needed to get __filename-based path resolution to work if you are loading in a shimmed CJS environment.

Replace import.meta.resolve with createRequire + path walking in getBundledCliPath(). The new implementation falls back to __filename when import.meta.url is unavailable (shimmed CJS environments like VS Code extensions bundled with esbuild format:"cjs"). Single ESM build output retained — no dual CJS/ESM builds needed. The fallback logic handles both native ESM and shimmed CJS contexts.

darthmolen · 2026-02-24T01:00:52Z

@SteveSandersonMS Good call — reworked to your suggestion.

What changed: Removed the dual CJS/ESM build entirely. The PR now keeps the single ESM build output unchanged (no modifications to esbuild-copilotsdk-nodejs.ts or package.json). The only change is in getBundledCliPath() which now falls back to __filename-based path resolution when import.meta.url is unavailable (shimmed CJS environments like VS Code extensions bundled with esbuild format:"cjs").

The fix:

createRequire(import.meta.url ?? pathToFileURL(__filename).href) handles both native ESM and shimmed CJS
Walks require.resolve.paths() instead of using import.meta.resolve() (which fails in CJS contexts)
Improved error message per the reviewer feedback (includes search path count)

Net diff from main: 2 files — src/client.ts (the function fix) and a new test/cjs-compat.test.ts.

Copilot AI review requested due to automatic review settings February 23, 2026 18:45

darthmolen requested a review from a team as a code owner February 23, 2026 18:45

Copilot started reviewing on behalf of darthmolen February 23, 2026 18:46 View session

Copilot AI reviewed Feb 23, 2026

View reviewed changes

darthmolen force-pushed the fix/528-cjs-compatibility branch from d1d3669 to fb560ad Compare February 24, 2026 00:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Comments

fix(nodejs): add CJS compatibility for VS Code extensions#546

fix(nodejs): add CJS compatibility for VS Code extensions#546
darthmolen wants to merge 1 commit intogithub:mainfrom
darthmolen:fix/528-cjs-compatibility

darthmolen commented Feb 23, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 23, 2026

Uh oh!

Copilot AI Feb 23, 2026

Uh oh!

Copilot AI Feb 23, 2026

Uh oh!

SteveSandersonMS commented Feb 23, 2026

Uh oh!

SteveSandersonMS commented Feb 23, 2026

Uh oh!

darthmolen commented Feb 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

-        // Verify that new CopilotClient({ cliUrl: "8080" }) doesn't throw
-        // (constructor calls getBundledCliPath() for the default cliPath)
-        const script = `
-            const sdk = require(${JSON.stringify(cjsEntryPoint)});
-            // Use cliUrl to avoid actually spawning the CLI,
-            // but the constructor still evaluates getBundledCliPath() for cliPath default
+        // Verify that new CopilotClient({ cliUrl: "8080" }) doesn't throw.
+        // In the current implementation, the constructor still resolves a default
+        // cliPath via getBundledCliPath(), even when cliUrl is provided, so this
+        // test exercises getBundledCliPath() in a pure CJS context.
+        //
+        // NOTE: This test assumes that the bundled @github/copilot CLI package is
+        // present in node_modules. It does NOT simulate environments (e.g. some
+        // VS Code extension bundles) where @github/copilot is absent.
+        const script = `
+            const sdk = require(${JSON.stringify(cjsEntryPoint)});
+            // Use cliUrl to avoid actually spawning the CLI process; we only care
+            // that constructing the client (and resolving the bundled CLI path)
+            // does not throw in a CJS-only environment.

Comments

Conversation

darthmolen commented Feb 23, 2026

Summary

Problem

Solution

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

SteveSandersonMS commented Feb 23, 2026

Uh oh!

SteveSandersonMS commented Feb 23, 2026

Uh oh!

darthmolen commented Feb 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants